Skip to content
This repository was archived by the owner on Jul 19, 2019. It is now read-only.

Conversation

@dsc8x
Copy link

@dsc8x dsc8x commented Jan 31, 2018

See #309

The current implementation only auto-highlights an item if the input matches the beginning of the item label.

That's problematic if your shouldItemRender implementation allows matching anywhere in the item label, not just from the start.

This PR fixes that.

const tree = mount(AutocompleteComponentJSX({}))
expect(tree.state('highlightedIndex')).toEqual(null)
tree.setProps({ value: 'a' })
tree.setProps({ value: 'ri' })
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The item we are matching here is Arizona.
In the old test we typed a, which put the Arizona item first and auto-highlighted it.

But if you typed ri then still Arizona was put as first item, but it was not auto-highlighted, since ri does not match the beginning of Arizona.

With the changes in this PR, Arizona will now correctly be highlighted.

const itemValueDoesMatch = (itemValue.toLowerCase().indexOf(
value.toLowerCase()
) === 0)
) > -1)
Copy link
Author

@dsc8x dsc8x Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue, that itemValueDoesMatch is even unnecessary.
If an item is displayed because it passed the check of shouldItemRender then we can also auto-highlight it if it's the first item in the list.

@CMTegner
Copy link
Collaborator

CMTegner commented Feb 4, 2018

The existing logic won't be changed at the moment. See #239 for explanation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants